✨ Add feature gates support for experimental API fields #4973
Conversation
k8s-ci-robot
added
the
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: wazery The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
cncf-cla: no
|
Hi @wazery. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
k8s-ci-robot
added
cncf-cla: yes
wazery
force-pushed
the
w-feature-gates
branch
2 times, most recently
from
f03050d to
00e9604
Compare
|
/ok-to-test |
k8s-ci-robot
added
ok-to-test
There was a problem hiding this comment.
Pull Request Overview
This PR implements feature gates support for experimental API fields in Kubebuilder controllers. The implementation enables developers to gradually rollout experimental features while maintaining backward compatibility, similar to Kubernetes core feature gates.
- Adds
+feature-gatemarker support for API fields with automatic discovery - Implements feature gate parsing, validation, and runtime control via
--feature-gatesflag - Integrates feature gates into project scaffolding and generation workflow
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
testdata/project-v4/api/v1/captain_types.go |
Adds example Bar field with experimental-bar feature gate marker |
test/e2e/v4/featuregates_test.go |
E2E tests for feature gate scaffolding and project building |
test/e2e/v4/featuregates_discovery_test.go |
Tests for feature gate discovery and parsing functionality |
pkg/plugins/golang/v4/scaffolds/internal/templates/makefile.go |
Adds TODO comment for future controller-gen feature gate support |
pkg/plugins/golang/v4/scaffolds/internal/templates/cmd/main.go |
Integrates feature gates flag and validation into main controller entry point |
pkg/plugins/golang/v4/scaffolds/internal/templates/cmd/featuregates.go |
Template for generating feature gates package with parsing and validation |
pkg/plugins/golang/v4/scaffolds/internal/templates/api/types.go |
Adds commented example of feature-gated field in generated API types |
pkg/plugins/golang/v4/scaffolds/init.go |
Includes feature gates scaffolding in initialization process |
pkg/plugins/golang/v4/scaffolds/api_test.go |
Unit tests for API scaffolder including feature gate discovery |
pkg/plugins/golang/v4/scaffolds/api.go |
Implements feature gate discovery and integration with scaffolding |
pkg/machinery/featuregate_test.go |
Unit tests for feature gate parsing and validation logic |
pkg/machinery/featuregate_marker_test.go |
Tests for feature gate marker parsing from Go source files |
pkg/machinery/featuregate_marker.go |
Parser for discovering +feature-gate markers in Go source code |
pkg/machinery/featuregate.go |
Core feature gate types and parsing functionality |
go.mod |
Adds testify dependency for improved testing |
docs/book/src/reference/markers/feature-gates.md |
Comprehensive documentation for feature gates usage |
docs/book/src/reference/markers.md |
Updates markers reference to include feature gates |
docs/book/src/SUMMARY.md |
Adds feature gates to documentation table of contents |
pkg/machinery/featuregate.go
Outdated
| case "true", "1", "yes", "on": | ||
| enabled = true | ||
| case "false", "0", "no", "off": | ||
| enabled = false | ||
| default: | ||
| return nil, fmt.Errorf("invalid feature gate value for %s: %s", name, value) |
There was a problem hiding this comment.
[nitpick] The parsing logic accepts 'yes', 'on', 'no', 'off' values but this is inconsistent with Kubernetes feature gate parsing which typically only accepts 'true' and 'false'. This could lead to confusion for users familiar with Kubernetes patterns.
| case "true", "1", "yes", "on": | |
| enabled = true | |
| case "false", "0", "no", "off": | |
| enabled = false | |
| default: | |
| return nil, fmt.Errorf("invalid feature gate value for %s: %s", name, value) | |
| case "true": | |
| enabled = true | |
| case "false": | |
| enabled = false | |
| default: | |
| return nil, fmt.Errorf("invalid feature gate value for %s: %s (must be 'true' or 'false')", name, value) |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
@camilamacedo86 what do you think about this, should we use only 'true' or 'false' as suggested?
k8s-ci-robot
added
the
needs-rebase
wazery
force-pushed
the
w-feature-gates
branch
3 times, most recently
from
f906d39 to
a188dd5
Compare
k8s-ci-robot
added
the
needs-rebase
| @@ -0,0 +1,233 @@ | |||
| /* | |||
There was a problem hiding this comment.
Note that these tests are not currently run in any CI.
Since they don’t interact with a cluster, they could be added to:
https://github.com/kubernetes-sigs/kubebuilder/blob/master/test/features.sh
However, if we want to actually validate that solutions with a feature gate are deployable—and not just test the scaffold like init—then we need a real e2e test.
In that case, we should set it here:
https://github.com/kubernetes-sigs/kubebuilder/blob/master/test/e2e/setup.sh#L67-L68
so it runs using a kind cluster in Prow.
I’ll need more time to review this approach in detail, but here’s how I think it should work:
- Create the project.
- Create the controller.
- Add a feature gate that outputs something like "experimental feature gate" in the logs.
- Build and deploy, patching to enable the feature gate.
- Validate that the logs contain the expected output.
This would give us proper end-to-end coverage for feature-gate behavior.
| return nil | ||
| } | ||
|
|
||
| const featureGatesTemplate = `{{ .Boilerplate }} |
There was a problem hiding this comment.
One of the challenges here is the UX.
A feature gate is an advanced capability, and not all projects will require or use it.
We need to think about how to make it optional.
One idea is to use plugins. For example, if we run:
kubebuilder edit plugins=feature-gate
then we could add the feature gate logic into the scaffolds.
We also have markers that can inject code, which might be useful here. For reference:
https://book.kubebuilder.io/reference/markers/scaffold
|
|
||
| const featureGatesTemplate = `{{ .Boilerplate }} | ||
|
|
||
| package featuregates |
There was a problem hiding this comment.
How is your idea?
Could we have a UX where we minimise the utilities in the scaffolds?
Could we make it simple for common use cases?
| @@ -0,0 +1,207 @@ | |||
| /* | |||
There was a problem hiding this comment.
I don’t think we should scaffold those for the project.
Instead, we should think about proposing feature gates in a way that is easy to use.
We also need to be careful not to overload projects with too many helpers.
We really avoid to do that, since it is hard to keep maintained and support as for the end users understand
See how I was thinking for this work:
#849 (comment)
There was a problem hiding this comment.
All logic to generate the CRD schemas must be in controller-runtime.
If you are doing it as an experiment, it's fine. But we cannot release and change, changes in the scaffold are burn for end users and we must be very careful,
There was a problem hiding this comment.
Thank you so much for starting to look into this — really great work! 🙌
I just wanted to share a couple of points to help us think about the approach and how this could work in practice.
The main thing to keep in mind is:
- Be careful with what we add by default.
- We cannot adding TODOs in the scaffolded code. The TODOs that exist means, the user will need to change that place ( you can search for
TODO(user)) - Don’t introduce unnecessary complexity for the most common cases.
Why this logic to generate the CRDs can’t be in Kubebuilder
We can’t add the marker to generate CRDs directly in Kubebuilder — that’s something that belongs in controller-runtime.
Kubebuilder’s role is to:
- Provide helpers (e.g., Makefile targets) for deploying manifests with feature gates.
- Provide docs to guide users on enabling/disabling features.
How the UX could work
Most users will want to test their solution in one of two ways:
- Production/Stable — no experimental features.
- All feature gates enabled — common for CI or experimental testing.
Less common: enabling/disabling specific feature gates (that seems useful for manual tests only)
Therefore, we could:
- Have a Makefile target to generate experimental manifests with all feature gates on.
- Let e2e tests apply those manifests.
- We will either need to think about how to distribute them with YAML and Helm charts. How that impac the current ways for users distribute their projects see: https://github.com/kubernetes-sigs/kubebuilder/blob/master/testdata/project-v4/README.md#project-distribution
If users want just one or a few feature gates, we can explain how in docs — no need for special helpers.
Common scenarios
- Production / Stable
- All feature flags enabled (CI/testing)
- Feature sets (Stable/Beta/Alpha) rather than one-by-one flags
This gives flexibility without making the default setup harder to use.
I hope that makes sense, please feel free to check out and let us know wdyt
This change improves the UX by making feature gate infrastructure opt-in rather than always generated. Key changes: - Added --with-feature-gates flag to init and create api commands - Made feature gate scaffolding conditional based on flag or auto-detection - Updated main.go template to conditionally include feature gate code - Added auto-detection for existing feature gate infrastructure - Created FeatureGateScaffolder interface for proper type handling The infrastructure is now only generated when: 1. Explicitly requested via --with-feature-gates flag, or 2. Auto-detected when existing feature gate infrastructure is found This addresses PR feedback suggesting that feature gate infrastructure should be optional to improve user experience.
Updated documentation to reflect the new optional feature gate infrastructure: - Updated feature-gates.md to explain optional nature and new CLI flags - Added note about --with-feature-gates flag in quick-start.md - Added examples to CLI help text for both init and create api commands - Clarified auto-detection behavior and when infrastructure is generated - Updated troubleshooting section with new flag usage The documentation now clearly explains: 1. Feature gates are opt-in rather than always generated 2. Three ways to enable: explicit flags, auto-detection, or marker discovery 3. Better UX through conditional scaffolding
- Remove manual feature gate additions from testdata files (auto-generated) - Fix unit test expectations to not expect markers in clean testdata - Update documentation to clarify testdata file handling - Ensure tests properly validate clean vs. modified testdata scenarios Addresses feedback from @camilamacedo86 in PR kubernetes-sigs#4973: - Testdata files are auto-generated and should not contain manual additions - E2E tests should create fresh projects and add markers programmatically - Unit tests should expect clean testdata without manual feature gate markers
wazery
force-pushed
the
w-feature-gates
branch
from
a1ae5ed to
4e1751c
Compare
k8s-ci-robot
added
do-not-merge/invalid-commit-message
|
Hi @camilamacedo86! 👋 Thank you for the detailed feedback so much, I truly appreciate them and how you cleared many points up for my understanding. I've addressed all the points you raised: 1. Made Feature Gates Optional
2. Cleaned Up Testdata Files
3. E2E Tests Use Fresh Projects
4. Kubernetes-Compatible Parsing (suggested by Copilot)
5. Simple UX Focus
Implementation Summary:Optional Infrastructure: # Explicit scaffolding
kubebuilder init --with-feature-gates
kubebuilder create api --with-feature-gates
# Auto-detection (when markers found or infrastructure exists)
kubebuilder create api # discovers +feature-gate markers automaticallyClean Integration:
All unit tests pass, e2e tests work correctly, and the implementation follows Kubebuilder's design principles. The approach prioritises simplicity while providing the flexibility needed for experimental features. Regarding the |
Updated feature gate tests to reflect the new optional behavior: - Added test to verify feature gates are NOT generated by default - Updated existing tests to use --with-feature-gates flag when needed - Fixed test expectations for auto-detection scenarios - Ensured tests properly validate both opt-in and discovery behavior Tests now correctly validate: 1. Default behavior: no feature gate infrastructure generated 2. Explicit flag: infrastructure generated when requested 3. Auto-detection: infrastructure generated when markers discovered
- Remove manual feature gate additions from testdata files (auto-generated) - Fix unit test expectations to not expect markers in clean testdata - Update documentation to clarify testdata file handling - Ensure tests properly validate clean vs. modified testdata scenarios
wazery
force-pushed
the
w-feature-gates
branch
from
af6dfa9 to
694284b
Compare
k8s-ci-robot
removed
the
do-not-merge/invalid-commit-message
There was a problem hiding this comment.
Hi @wazery
Auto-detection works when existing infrastructure is found OR +feature-gate markers are discovered
No scaffolding by default - keeps projects clean for common use cases
The problem is that the end-users' project should not have the code for the markers.
The makers should be implemented in the tools and be available for all of them
In this case, because it is related to how the CRDs are generated, this marker should be implemented in controller-tools instead of kubebuilder.
I hope that makes sense. WDYT about pushing a PR with the code to controller-tools?
|
@wazery: The following tests failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
✨ Add feature gates support for experimental API fields
Description
Implements initial feature gates functionality to enable/disable experimental features in Kubebuilder controllers. Adds
+feature-gatemarker support for API fields, automatic discovery during scaffolding, and--feature-gatesflag for runtime control.Motivation
Enables gradual rollout of experimental features while maintaining backward compatibility. Similar to Kubernetes core feature gates, this allows developers to safely introduce new API fields and functionality.
Changes
pkg/machinery/featuregate.go)pkg/machinery/featuregate_marker.go)--feature-gatesflag to controller main.goFuture Work
CRD schema modification requires controller-tools support (issue #1238). When implemented,
+kubebuilder:feature-gatemarkers will automatically exclude disabled fields from CRD schemas.Fixes #849